Skip to content

Save response form documents as draft assets until publication v2 [WHIT-3750] - #11764

Open
eYinka wants to merge 8 commits into
mainfrom
save-response-form-docs-as-draft-2
Open

Save response form documents as draft assets until publication v2 [WHIT-3750]#11764
eYinka wants to merge 8 commits into
mainfrom
save-response-form-docs-as-draft-2

Conversation

@eYinka

@eYinka eYinka commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Alternative solution to #11755


What

Consultation and Call for Evidence response documents are now stored as draft assets in Asset Manager while the owning edition is still a draft or access-limited, and are flipped to live only when the edition is published; which matches how file attachments already behave.

Why

By design, response documents were stored as live assets in Asset Manager from the moment they were uploaded, regardless of whether the owning edition was still a draft or access-limited. File attachments, by contrast, have always been stored as draft and flipped to live on publish. This meant response documents were reachable by anyone with the asset URL before the document was ever published.

This change brings response documents in line with file attachments i.e draft/access-limited editions' response documents are no longer publicly retrievable before publication, and the same preview link that grants access to a draft edition also inherently grants access to its response document.

Before/After

What Before After
Asset Manager state on upload draft: false - live immediately draft: true - draft until the edition is published
Storage engine Storage::PreviewableStorage Storage::AttachmentStorage
Access limiting Not applied to response document assets access_limited_organisation_ids / access_limited_user_ids are set from the parent edition
Preview token (auth_bypass_id) Did not gate response document assets (they were already live) Draft response document assets are gated by the token, same as attachments
On publish No mechanism existed to flip response document assets to live - they were always live AttachmentAssetPublisher flips response document assets from draft to live, mirroring the existing attachment/image flow
Asset Manager metadata sync (parent_document_url, draft state) Skipped entirely -PreviewableStorage never passed the owning edition through, so AssetManagerAttachmentMetadataJob never ran for response documents Runs after every upload, same as attachments/images

JIRA


⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

This application is owned by the Whitehall Experience team. Please let us know in #govuk-whitehall-experience-tech when you raise any PRs.

Follow these steps if you are doing a Rails upgrade.

Response documents need to know which edition (and therefore which access controls) they belong to, so Asset Manager can gate them the
same way file attachments already are. This adds the accessor only -
to be used by future commits.
.map(&:name)

asset_data_types.each(&block)
%w[AttachmentData ImageData].each(&block)

@eYinka eYinka Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AssetData is auto-discovered by this shared test helper
(for_each_asset_data_type) and it previously scanned by module inclusion. Since
Consultation/CallForEvidence response form models now include AssetData too, that scan was changed to an explicit %w[AttachmentData ImageData] list to avoid sweeping response form data into shared tests built for a generic edition. Hopefully this is okay?

@eYinka
eYinka force-pushed the save-response-form-docs-as-draft-2 branch from 776e054 to 0df0e71 Compare September 4, 2026 11:50

@ChrisBAshton ChrisBAshton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🎉 Thanks for iterating this PR from the other one, I do really like the fact that we're converging onto one way of doing assets.

Some thoughts below, will approve when ready (since it would need a re-approval post-rebase anyway)

Comment thread test/attachment_test_helper.rb
Comment thread test/unit/app/models/consultation_response_form_test.rb
Comment thread app/sidekiq/asset_manager_attachment_metadata_job.rb
Comment thread app/services/service_listeners/draft_attachment_asset_discarder.rb
This is prep work for reusing AssetData in ConsultationResponseFormData/
CallForEvidenceResponseFormData. AssetData expects every item in its attachments collection to respond to `#attachable` and `#deleted?`, and expects the including model to define a `#replaced?` behaviour itself - which only comes from a separate Replaceable concern.
This adds those methods to ConsultationResponseForm/CallForEvidenceResponseForm and ConsultationResponseFormData/CallForEvidenceResponseFormData, all returning sensible fixed values
since response documents have no soft-delete or replacement concept.
No behaviour change yet and nothing calls these until further commits.
Consultation and call for evidence response documents were always uploaded as live assets in Asset Manager, even while the owning edition was still a draft or access-limited. This commit switches them to use the same draft/access-limited storage flow that File attachments use, so they're gated by the parent edition's access controls until it is published.

Note: `AssetData` is auto-discovered by a shared test helper
(`for_each_asset_data_type`) in `test/attachment_test_helper.rb` and it previously scanned by module inclusion. Since Consultation/CallForEvidence response form models now include `AssetData` too, that scan was changed to an explicit `%w[AttachmentData ImageData]` list to avoid sweeping response form data into shared tests built for generic editions.
Response form data models have no `replaced_by_id` column / Replaceable concern (unlike AttachmentData/ImageData), so AssetManagerAttachmentMetadataJob's `replaced_by` lookup would raise an error for them whenever their parent documents are updated.
I've now added a guard to the lookup so that it skips the replacement operations for such models.
This flips response document assets from draft to live once the owning
Consultation/CallForEvidence is published. It mirrors how
AttachmentAssetPublisher already does this for file attachments and
images.

There's a trade-off though: this couples the otherwise edition-agnostic publisher to Consultation/CallForEvidence's specific association chains. Probably okay, pending when we migrate these document types to standard edition.  The same trade-off already exists in `EditionAuthBypassAssetPropagator` which branches on the same two edition types for the same reason, so this follows the existing approach.
…dits

Previously, changing a consultation/call for evidence's access-limiting after its response document was uploaded never propagated to the asset in Asset Manager since `AssetManagerAttachmentMetadataJob` only re-ran at upload time. This is because `ServiceListeners::AttachmentUpdater`
(which re-syncs on every edition save) had no branch for response form
data, only Attachment/Image. I've now added that branch and also included some explicit tests for that behaviour.
…eleted

DraftAttachmentAssetDiscarder was built to only handle Attachment/Image, so a draft Consultation/CallForEvidence that is deleted before ever being published would leave its response document as an orphaned draft asset in Asset Manager.

AssetData's `needs_discarding?` (attachments.size == 1) gives us an ideal answer for response form data. i.e The attachments method on a Response Form Data always returns exactly one item (the response form, or Attachment::Null as a fallback). Therefore, it's always safe to discard; unlike in AttachmentData/ImageData, where size > 1 would mean the same data is still referenced by another (possibly) live attachment.
Storage::PreviewableStorage was only ever used by ImageUploader and ResponseDocumentUploader. Both have since moved to Storage::AttachmentStorage (images in #11667, response documents in this PR), so nothing references PreviewableStorage anymore.
@eYinka
eYinka force-pushed the save-response-form-docs-as-draft-2 branch from 0df0e71 to 8276923 Compare September 7, 2026 12:33

@ChrisBAshton ChrisBAshton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good ⭐ (Assuming it's been put through its paces on integration)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants